Skip to content

Remove SharedCache::getNumericStorageId to let CacheWrapper do it#4004

Merged
rullzer merged 3 commits into
masterfrom
backport-27172
Mar 28, 2017
Merged

Remove SharedCache::getNumericStorageId to let CacheWrapper do it#4004
rullzer merged 3 commits into
masterfrom
backport-27172

Conversation

@MorrisJobke

Copy link
Copy Markdown
Member

The CacheWrapper will properly forward the call to the wrapped cache.

Signed-off-by: Morris Jobke <hey@morrisjobke.de>

@icewind1991 icewind1991 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👎 For removing it from the shared cache

Having the tests from the rest of the commit would be nice though

return $this->cache;
}

public function getNumericStorageId() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing this breaks the sharing performance improvements

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@MorrisJobke

Copy link
Copy Markdown
Member Author

Unit tests fail:

1) OCA\Files_Sharing\Tests\CacheTest::testNumericStorageId
Failed asserting that false matches expected 344.
/drone/src/github.com/nextcloud/server/apps/files_sharing/tests/CacheTest.php:550

Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991

Copy link
Copy Markdown
Member

Fixed

@codecov-io

Copy link
Copy Markdown

Codecov Report

Merging #4004 into master will decrease coverage by 0.09%.
The diff coverage is 71.42%.

@@             Coverage Diff             @@
##             master    #4004     +/-   ##
===========================================
- Coverage     54.33%   54.24%   -0.1%     
- Complexity    21167    21282    +115     
===========================================
  Files          1305     1310      +5     
  Lines         80829    81201    +372     
  Branches       1282     1284      +2     
===========================================
+ Hits          43920    44044    +124     
- Misses        36909    37157    +248
Impacted Files Coverage Δ Complexity Δ
apps/files_sharing/lib/SharedStorage.php 76.66% <100%> (+1.28%) 90 <1> (+3) ⬆️
apps/files_sharing/lib/Cache.php 90% <100%> (+0.2%) 20 <0> (ø) ⬇️
lib/private/Files/Cache/Cache.php 82.85% <50%> (-0.31%) 103 <0> (+4)
apps/files/lib/App.php 27.27% <0%> (-47.73%) 2% <0%> (ø)
lib/private/NavigationManager.php 53.03% <0%> (-33.54%) 39% <0%> (+13%)
...atenotification/lib/Notification/BackgroundJob.php 66.66% <0%> (-18.19%) 30% <0%> (+8%)
lib/private/legacy/template.php 30.34% <0%> (-10.91%) 36% <0%> (+1%)
.../federation/lib/Middleware/AddServerMiddleware.php 86.66% <0%> (-5.65%) 4% <0%> (+1%)
lib/private/Updater/VersionCheck.php 92.3% <0%> (-4.99%) 7% <0%> (+1%)
apps/files/lib/Controller/ViewController.php 75.96% <0%> (-4.96%) 17% <0%> (ø)
... and 52 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 39ac804...ae30169. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants